Skip to content

Schemas validation while running base44 dev#395

Merged
artemdemo merged 6 commits intomainfrom
schemas-validation
Mar 16, 2026
Merged

Schemas validation while running base44 dev#395
artemdemo merged 6 commits intomainfrom
schemas-validation

Conversation

@artemdemo
Copy link
Contributor

@artemdemo artemdemo commented Mar 9, 2026

Note

Description

This PR adds entity record validation to the dev server, introducing a Validator class that enforces entity schemas on create and update operations. When a record fails validation (wrong types, missing required fields), the API now returns a 422 Unprocessable Entity response with a structured error object — matching the shape of production API validation errors. The Database class is also reorganized into a db/ subdirectory to better group schema-aware logic.

Related Issue

None

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactoring (no functional changes)
  • Other (please describe):

Changes Made

  • Added packages/cli/src/cli/dev/dev-server/db/validator.ts — new Validator class with filterFields(), applyDefaults(), and validate() methods supporting all Base44 field types (string, integer, number, boolean, array, object), required-field checks, partial validation, and recursive nested type validation
  • Added packages/cli/src/cli/dev/dev-server/db/database.ts — refactored Database class (moved from database.ts) that now stores entity schemas and exposes validate() and prepareRecord() methods
  • Deleted packages/cli/src/cli/dev/dev-server/database.ts — replaced by db/database.ts
  • Updated routes/entities.ts — POST (single and bulk) and PATCH handlers now call db.prepareRecord() + db.validate() before inserting/updating; invalid records return 422 with a ValidationError body
  • Updated core/resources/entity/schema.ts — exported PropertyDefinition type, made type field required (was optional), and cleaned up recursive Zod type annotations
  • Added packages/cli/tests/scripts/validator.spec.ts — comprehensive unit test suite for the Validator class covering all field types, required fields, partial validation, nested arrays/objects, defaults, and error shapes
  • Minor: fixed vitest.config.ts to use node:path import; added biome-ignore comment in connector test

Testing

  • I have tested these changes locally
  • I have added/updated tests as needed
  • All tests pass (npm test)

Checklist

  • My code follows the project's style guidelines
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation (if applicable)
  • My changes generate no new warnings
  • I have updated docs/ (AGENTS.md) if I made architectural changes

Additional Notes

The validator intentionally silently ignores unknown fields (relying on filterFields() to strip them beforehand) and skips items/properties sub-validation when those schema keys are absent, with inline test comments noting this matches current production behavior.


🤖 Generated by Claude | 2026-03-11 00:00 UTC

@@ -1,5 +1,5 @@
import { resolve } from "node:path";
import { defineConfig } from "vitest/config";
import { resolve } from "path";
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not related to PR, just cleaning up some linting

@github-actions
Copy link
Contributor

github-actions bot commented Mar 9, 2026

🚀 Package Preview Available!


Install this PR's preview build with npm:

npm i @base44-preview/cli@0.0.44-pr.395.5afac9a

Prefer not to change any import paths? Install using npm alias so your code still imports base44:

npm i "base44@npm:@base44-preview/cli@0.0.44-pr.395.5afac9a"

Or add it to your package.json dependencies:

{
  "dependencies": {
    "base44": "npm:@base44-preview/cli@0.0.44-pr.395.5afac9a"
  }
}

Preview published to npm registry — try new features instantly!

@base44 base44 deleted a comment from claude bot Mar 10, 2026
@artemdemo artemdemo force-pushed the schemas-validation branch from 847df50 to e91f0a6 Compare March 11, 2026 10:20
@base44 base44 deleted a comment from claude bot Mar 11, 2026
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just moved to a new location, inside db directory

delete: RLSRuleSchema.optional(),
});

const PropertyDefinitionSchema: z.ZodType<unknown> = z.looseObject({
Copy link
Contributor Author

@artemdemo artemdemo Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see reason for it to be unknown when types are casted.
(same below)
Am i I missing something?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you're right

@base44 base44 deleted a comment from claude bot Mar 11, 2026
@artemdemo
Copy link
Contributor Author

@claude review

@claude
Copy link

claude bot commented Mar 11, 2026

Claude finished @artemdemo's task in 1m 56s —— View job


PR Review: Schemas Validation

  • Gather context and understand PR
  • Read changed files
  • Analyze code quality, correctness, and patterns
  • Post review feedback

Overall this is a clean, well-structured addition. The Validator class is focused, recursive validation works correctly for nested arrays/objects, and the test coverage is thorough. A few issues worth addressing:


Issues

1. null values fail type validation silently-but-wrongly

validateValue does not handle null explicitly. If a client sends { "name": null } to clear an optional string field, the validator rejects it with "Input should be a valid string" — because typeof null === "object", not "string". For the object case the null guard returns an error too (line 162). This is likely unintentional; nulls are common for clearing optional fields.

File: packages/cli/src/cli/dev/dev-server/db/validator.ts:116

Suggestion: return early with { hasError: false } before the switch if value === null (or align with production API behaviour for null).

Fix this →


2. Misleading error for unsupported schema types

When a property's type field contains an unsupported value (e.g. "date"), the error returned is "Input should be a valid date" — this reads as if the user's value is wrong, when actually the schema type itself is unrecognised.

File: packages/cli/src/cli/dev/dev-server/db/validator.ts:129-135

// current
`Error in field ${fieldPath}: Input should be a valid ${propertyType}`

// clearer
`Error in field ${fieldPath}: Unsupported field type "${propertyType}"`

3. Bulk create doesn't identify which record failed validation

POST /:entityName/bulk validates each record in a loop but the error response doesn't include which index failed. A client sending 10 records has no way to know which one caused the 422.

File: packages/cli/src/cli/dev/dev-server/routes/entities.ts:253-260

Consider including the record index in the validation error message, or at minimum in a wrapper field (e.g. { index: i, ...validation.error }).


4. validateRequiredFields treats null as missing

record[required] == null (loose equality) catches both undefined and null. If a client explicitly sends null for a required field on a create, the error will say "Field required" rather than a type error — which may be the right behaviour, but is worth an explicit test.

File: packages/cli/src/cli/dev/dev-server/db/validator.ts:216


Nits

  • vitest.config.ts:9testTimeout is missing indentation (4 spaces). The PR description says this file change is unrelated lint cleanup, but the formatting was made worse.
  • validator.ts:25-32 — The fieldTypes array and the switch statement below are loosely coupled. Adding a new type to fieldTypes (e.g. "null") won't automatically add handling in the switch, making default the fallback. A comment noting this relationship would help.
  • database.ts:33validate and prepareRecord both throw Error if the schema is missing, but withCollection already guards against unknown entity names, so this code path is unreachable in practice. A comment or a /* istanbul ignore next */ would clarify intent.

What's good

  • The filterFieldsapplyDefaultsvalidate ordering in prepareRecord/route handlers is correct.
  • partial: true for PUT correctly skips required-field checks while still validating types.
  • Recursive validation for nested arrays-of-objects works correctly (confirmed by the entries[0].count test).
  • ValidationError response shape (error_type, message, request_id, traceback) intentionally mirrors production — good for parity.
  • The Database class cleanly encapsulates both the NeDB datastore and the schema store; load/dropAll are symmetric.
  • Test coverage is comprehensive; the "temporary" skips are honestly documented.

| Branch: schemas-validation

@artemdemo artemdemo changed the title Schemas validation Schemas validation during dev Mar 11, 2026
@artemdemo artemdemo changed the title Schemas validation during dev Schemas validation while running base44 dev Mar 11, 2026
@artemdemo artemdemo marked this pull request as ready for review March 11, 2026 14:03
"object",
];

export class Validator {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a high level idea, haven't dived into it -
But as far as i understand the entities schema are basically json-schema standard, and i saw this package @cfworker/json-schema (https://github.com/cfworker/cfworker) which we can feed the schema to and then just call validate?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The thing is that leaders don't really want to validate - this schema is just documentation of intent
And looks we're going with this approach forward.
json-schema supports far more features than what we need to handle.
Therefore not sure if need to introduce anything that will actually use this schema to validate any further than just types.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looked into it once more.
It's possible to use json-schema, not sure if makes code simpler though.
I will still need to do recursions in order to make json-schema work with limited set of validations.
And also I will need to filter out errors that are not relevant to our case.

If you want to take a look, here is a prompt that I used to test it out:

Take a look at how I validate using a schema. Basically, I am only interested in the property type and whether it's required or not. I don't validate other constraints like maxLength, pattern, minLength, etc. I do validate recursively, though. If a property is an object or an array, I will go and check its properties/items, but will only validate the same two things: type and whether it's required. So, I'm thinking whether I can use `@cfworker/json-schema` instead for the same thing. This package does much more, but maybe I can silence other errors or something.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should proceed with the current implentation considering the links you added, I just don't love maintaining this validation logic (and issues that may raise) on the CLI side.. but it looks kinda simple for now 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we will need anyway maintain validation logic in some capacity 😞

Copy link
Collaborator

@kfirstri kfirstri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good, what do you think about https://github.com/cfworker/cfworker as a validator?

@github-project-automation github-project-automation bot moved this from Backlog to In review in CLI Development Mar 16, 2026
@artemdemo artemdemo merged commit 9435f39 into main Mar 16, 2026
26 checks passed
@artemdemo artemdemo deleted the schemas-validation branch March 16, 2026 10:36
@github-project-automation github-project-automation bot moved this from In review to Done in CLI Development Mar 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants